checkout: Provide useful error with checkout -H and incompat mode

Previously we'd assert and dump core if one used `checkout -H` without
`-U` on a bare-user repo, because we'd hit the bare-user symlink case.

Rework the code to handle this, and add tests. I hit this when I was going to
suggest to someone to use `-H` to ensure they were getting hardlinks.

Closes: #779
Approved by: jlebon

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index 09966a94fd274c56961ed1ec9a9b900ed87f80c6..392e16fda677837a3a2958b23739194ccb0103ea 100644 (file)
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -423,6 +423,7 @@ checkout_one_file_at (OstreeRepo                        *repo,
   gboolean ret = FALSE;
   const char *checksum;
   gboolean is_symlink;
+  gboolean is_bare_user_symlink = FALSE;
   gboolean can_cache;
   gboolean need_copy = TRUE;
   char loose_path_buf[_OSTREE_LOOSE_PATH_MAX];
@@ -431,7 +432,6 @@ checkout_one_file_at (OstreeRepo                        *repo,
   gboolean is_whiteout;
 
   is_symlink = g_file_info_get_file_type (source_info) == G_FILE_TYPE_SYMBOLIC_LINK;
-
   checksum = ostree_repo_file_get_checksum ((OstreeRepoFile*)source);
 
   is_whiteout = !is_symlink && options->process_whiteouts &&
@@ -468,20 +468,42 @@ checkout_one_file_at (OstreeRepo                        *repo,
 
       while (current_repo)
         {
-          gboolean is_bare = ((current_repo->mode == OSTREE_REPO_MODE_BARE
-                               && options->mode == OSTREE_REPO_CHECKOUT_MODE_NONE) ||
-                              (current_repo->mode == OSTREE_REPO_MODE_BARE_USER
-                               && options->mode == OSTREE_REPO_CHECKOUT_MODE_USER
-                               /* NOTE: bare-user symlinks are not stored as symlinks */
-                               && !is_symlink) ||
-                              (current_repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY
-                               && options->mode == OSTREE_REPO_CHECKOUT_MODE_USER));
+          /* TODO - Hoist this up to the toplevel at least for checking out from
+           * !parent; don't need to compute it for each file.
+           */
+          gboolean repo_is_usermode =
+            current_repo->mode == OSTREE_REPO_MODE_BARE_USER ||
+            current_repo->mode == OSTREE_REPO_MODE_BARE_USER_ONLY;
+          /* We're hardlinkable if the checkout mode matches the repo mode */
+          gboolean is_hardlinkable =
+            (current_repo->mode == OSTREE_REPO_MODE_BARE
+             && options->mode == OSTREE_REPO_CHECKOUT_MODE_NONE) ||
+            (repo_is_usermode && options->mode == OSTREE_REPO_CHECKOUT_MODE_USER);
+          gboolean is_bare = is_hardlinkable && !is_bare_user_symlink;
           gboolean current_can_cache = (options->enable_uncompressed_cache
                                         && current_repo->enable_uncompressed_cache);
           gboolean is_archive_z2_with_cache = (current_repo->mode == OSTREE_REPO_MODE_ARCHIVE_Z2
                                                && options->mode == OSTREE_REPO_CHECKOUT_MODE_USER
                                                && current_can_cache);
 
+          /* NOTE: bare-user symlinks are not stored as symlinks; see
+           * https://github.com/ostreedev/ostree/commit/47c612e5a0688c3452a125655a245e8f4f01b2b0
+           * as well as write_object().
+           */
+          is_bare_user_symlink = (repo_is_usermode && is_symlink);
+
+          /* Verify if no_copy_fallback is set that we can hardlink, with a
+           * special exception for bare-user symlinks.
+           */
+          if (options->no_copy_fallback && !is_hardlinkable && !is_bare_user_symlink)
+            {
+              glnx_throw (error,
+                          repo_is_usermode ?
+                          "User repository mode requires user checkout mode to hardlink" :
+                          "Bare repository mode cannot hardlink in user checkout mode");
+              goto out;
+            }
+
           /* But only under these conditions */
           if (is_bare || is_archive_z2_with_cache)
             {
@@ -596,7 +618,13 @@ checkout_one_file_at (OstreeRepo                        *repo,
   /* Fall back to copy if we couldn't hardlink */
   if (need_copy)
     {
-      g_assert (!options->no_copy_fallback);
+      /* Bare user mode can't hardlink symlinks, so we need to do a copy for
+       * those. (Although in the future we could hardlink inside checkouts) This
+       * assertion is intended to ensure that for regular files at least, we
+       * succeeded at hardlinking above.
+       */
+      if (options->no_copy_fallback)
+        g_assert (is_bare_user_symlink);
       if (!ostree_repo_load_file (repo, checksum, &input, NULL, &xattrs,
                                   cancellable, error))
         goto out;

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index 27b790266f624babcd519eb4aaee62514e1aa94a..294854bf4487dbafa6a2c2ff0505b846604545d2 100644 (file)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -38,9 +38,45 @@ if grep bare-user-only repo/config; then
     CHECKOUT_U_ARG="-U"
 fi
 
+validate_checkout_basic() {
+    (cd $1;
+     assert_has_file firstfile
+     assert_has_file baz/cow
+     assert_file_has_content baz/cow moo
+     assert_has_file baz/deeper/ohyeah
+     )
+}
+
 $OSTREE checkout test2 checkout-test2
+validate_checkout_basic checkout-test2
 echo "ok checkout"
 
+# Note this tests bare-user *and* bare-user-only
+rm checkout-test2 -rf
+if grep bare-user repo/config; then
+    $OSTREE checkout -U -H test2 checkout-test2
+else
+    $OSTREE checkout -H test2 checkout-test2
+fi
+validate_checkout_basic checkout-test2
+rm checkout-test2 -rf
+# Only do these tests on bare-user/bare, not bare-user-only
+# since the latter automatically synthesizes -U if it's not passed.
+if ! grep -q bare-user-only repo/config; then
+if grep -q bare-user repo/config; then
+    if $OSTREE checkout -H test2 checkout-test2 2>err.txt; then
+        assert_not_reached "checkout -H worked?"
+    fi
+    assert_file_has_content err.txt "User repository.*requires.*user"
+else
+    if $OSTREE checkout -U -H test2 checkout-test2 2>err.txt; then
+        assert_not_reached "checkout -H worked?"
+    fi
+    assert_file_has_content err.txt "Bare repository mode cannot hardlink in user"
+fi
+fi
+echo "ok checkout -H"
+
 $OSTREE rev-parse test2
 $OSTREE rev-parse 'test2^'
 $OSTREE rev-parse 'test2^^' 2>/dev/null && fatal "rev-parse test2^^ unexpectedly succeeded!"
@@ -64,13 +100,9 @@ ostree_repo_init test-repo --mode=bare-user
 rm test-repo -rf
 echo "ok repo-init on existing repo"
 
+rm checkout-test2 -rf
+$OSTREE checkout test2 checkout-test2
 cd checkout-test2
-assert_has_file firstfile
-assert_has_file baz/cow
-assert_file_has_content baz/cow moo
-assert_has_file baz/deeper/ohyeah
-echo "ok content"
-
 rm firstfile
 $OSTREE commit ${COMMIT_ARGS} -b test2 -s delete